Skip to content

feat: Add global shortcut to export logs#2336

Merged
ericlln merged 17 commits intodeephaven:mainfrom
ericlln:DH-18341-method-to-export-logs-when-login-or-connection-fails
Jan 17, 2025
Merged

feat: Add global shortcut to export logs#2336
ericlln merged 17 commits intodeephaven:mainfrom
ericlln:DH-18341-method-to-export-logs-when-login-or-connection-fails

Conversation

@ericlln
Copy link
Copy Markdown
Contributor

@ericlln ericlln commented Jan 9, 2025

  • Added Ctrl+Alt+Shift+L/Cmd-Option-Shift-L as a global shortcut for exporting support logs
  • When pressed in code-studio, it will download logs with the same contents as logs produced by pressing the 'Export Logs' button in the settings menu
  • When pressed in other contexts: when not logged in, on a reconnect error, or in embed-widgets, the logs are exported without the plugin and server info
  • Tested by E2E tests that navigate to the various contexts, presses the shortcut, and checks if a file was downloaded. Can further inspect the file downloaded in the tests but unsure if that's necessary

Closes #1963

@ericlln ericlln requested a review from mofojed January 9, 2025 15:03
@ericlln ericlln self-assigned this Jan 9, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 9, 2025 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@ericlln
Copy link
Copy Markdown
Contributor Author

ericlln commented Jan 9, 2025

I have read the CLA Document and I hereby sign the CLA

deephaven-internal added a commit to deephaven/cla that referenced this pull request Jan 9, 2025
@deephaven-internal
@ericlln has signed the CLA in deephaven/web-client-ui#2336
b94f2c5
mofojed
mofojed requested changes Jan 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the e2e test is failing in CI because the timeout may not be long enough - it may take a while for the UI to load in a resource constrained environment. You can increase the timeout for this call specifically by passing in the option: https://playwright.dev/docs/test-timeouts

Comment thread packages/embed-widget/vite.config.ts
Comment thread packages/code-studio/src/main/App.tsx
Comment thread packages/app-utils/src/components/AppBootstrap.tsx Outdated
Comment thread packages/app-utils/src/components/AppBootstrap.tsx Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 10, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 46.70%. Comparing base (6a08198) to head (e8c092f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...p-utils/src/utils/createExportLogsContextAction.ts 40.00% 3 Missing ⚠️
packages/code-studio/src/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #2336    +/-   ##
========================================
  Coverage   46.70%   46.70%            
========================================
  Files         704      705     +1     
  Lines       39044    39051     +7     
  Branches     9757     9943   +186     
========================================
+ Hits        18234    18240     +6     
- Misses      20799    20800     +1     
  Partials       11       11
Flag Coverage Δ
unit 46.70% <55.55%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ericlln ericlln requested a review from mofojed January 14, 2025 14:46
mofojed
mofojed previously approved these changes Jan 14, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great and resolves the ticket as specified! Going forward we can improve these error situations by having a visible button so the user doesn't need to remember a keyboard shortcut.

Comment thread tests/shortcuts.spec.ts Outdated
Comment thread tests/shortcuts.spec.ts Outdated
@ericlln @mofojed
Fix typo in tests/shortcuts.spec.ts
61c8b05 
Co-authored-by: Mike Bender <mikebender@deephaven.io>
@ericlln @mofojed
put comment into description of test.slow()
e8c092f 
Co-authored-by: Mike Bender <mikebender@deephaven.io>
@ericlln ericlln requested a review from mofojed January 14, 2025 23:40
@ericlln ericlln merged commit 6e813fd into deephaven:main Jan 17, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mofojed mofojed mofojed approved these changes

Assignees

@ericlln ericlln

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No way to export support logs before logging in or after there's a connection error

2 participants

@ericlln @mofojed
⚡